Skip to content

fix(dendrogram, v1.2): pass the color threshold to scipy as a number - #7291

Closed
github-actions[bot] wants to merge 1 commit into
release/v1.2from
backport/7234-pass-the-color-threshold-to-scipy-as-a-n-v1.2
Closed

fix(dendrogram, v1.2): pass the color threshold to scipy as a number#7291
github-actions[bot] wants to merge 1 commit into
release/v1.2from
backport/7234-pass-the-color-threshold-to-scipy-as-a-n-v1.2

Conversation

@github-actions

@github-actions github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Automated backport of #7234 to release/v1.2.

Source: 351ce20 · automation run

Any related issues, documentation, discussions?

Backport of #7234. Originally linked #7232.

How was this PR tested?

Release-branch CI runs on this branch once the conflicts are resolved and this PR is marked ready for review.

Was this PR authored or co-authored using generative AI tooling?

No.

### What changes were proposed in this PR?

Dendrogram's Color Threshold could not be set at all:

- the field was declared as a string and spliced into the generated
Python as a decode expression, so scipy received `'3'` rather than `3`
and raised `UFuncTypeError` comparing it against the linkage distances
- the only values that plotted were a blank field and the literal
`default`, and scipy documents those as equivalent — both mean 0.7 × max
distance — so nothing a user could type ever changed the coloring

This PR declares the field as `Option[Double]`:

- the number is spliced as a literal, so scipy gets a number
- an unset threshold stays `None`, the same 0.7 × max distance a blank
field already meant
- `@JsonDeserialize(contentAs = ...)` names the boxed class: Scala
erases `Option`'s element type, so without it Jackson leaves the raw
JSON value inside the Option and the first use throws
`ClassCastException`, and the primitive class would read a blank as 0 —
every link colored the same rather than "unset"

Compatibility: a numeric string saved earlier still reads as a number,
and those workflows were failing before this change anyway. A workflow
that stored the literal `default` no longer loads; clearing the field
plots the identical chart.

### Any related issues, documentation, discussions?

Fixes #7232.

### How was this PR tested?

- the operator's existing spec updated: a configured threshold is
asserted to reach the template as `color_threshold=42.5`, not as a
decoded string
- deserialization tests for a JSON number, a numeric string, blank, null
and absent — plus one that uses the value as a number, the case a round
trip cannot catch
- ran the generated Python against a pandas DataFrame:
`color_threshold=3.0` plots a figure and so does the unset case, while
passing the same value as a string, which is what the operator does
today, raises `UFuncTypeError`
- whole workflow-operator module: 2020 tests passing, `scalafmtCheck`
clean

### Was this PR authored or co-authored using generative AI tooling?

(backported from commit 351ce20)

Generated-by: Claude Code (claude-opus-5[1m])
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

The cherry-pick conflicted and was committed with conflict markers. Resolve the conflicts on this branch, then mark this PR ready for review.

Conflicting files:

  • common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/dendrogram/DendrogramOpDesc.scala
  • common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/dendrogram/DendrogramOpDescSpec.scala

@Yicong-Huang

Copy link
Copy Markdown
Contributor

@kz930 can you take care of this PR?

@kz930

kz930 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

The cherry-pick conflict is resolved, but the branch lives in this repository and I do not have push access, so I opened #7306 from my fork with the resolved commit. Feel free to close this one.

@Yicong-Huang

Copy link
Copy Markdown
Contributor

The cherry-pick conflict is resolved, but the branch lives in this repository and I do not have push access, so I opened #7306 from my fork with the resolved commit. Feel free to close this one.

I see. Closing this PR in favor of #7306

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants